[Logging] Reducing stack usage by moving dmlc::LogMessageFatal
to the heap
#613
+6
−4
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When working on
tvm::runtime::Array
, we found thatCHECK
causes significant regression in stack utilization. This is not acceptable especially in compilers written in recursive visitor pattern, which crashes on comparably deep neural networks.As diving deeper into this case, we figured out that
dmlc::LogMessageFatal
is the cause - it is super sophisticated and throws inside the destructor.So we would love to patch dmlc-core, move
dmlc::LogMessageFatal
from stack to heap. A simple trick is to wrap it withstd::unique_ptr
- it is doable because dmlc-core has been updated to C++ 11.Besides patching dmlc-core, we can change the implementation of
tvm::runtime::Array
as well as follows:0) no change (using CHECK)
Below are the benchmarks we did. We use
g++ -fstack-usage
to dump the stack usage information for the target function.Settings:
virtual tvm::relay::Expr tvm::relay::CommonSubexprEliminator::VisitExpr_(const tvm::relay::CallNode*)
, which originally crashed this PR on a very deep networkResults:
Side notes: It is unclear right now why throwing
dmlc::Error
takes more stack space thanstd::runtime_error
, although they intent to be identical.See also: apache/tvm#5585 (comment)